-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move files_external mount config to the database #20370
Conversation
<name>*dbprefix*external_mounts</name> | ||
<declaration> | ||
<field> | ||
<name>mount_id</name> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
id is enough
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to keep the column names a bit more descriptive to make the queries more readable
cb18d1a
to
8515031
Compare
bdcf061
to
df53bc7
Compare
Added migration, no tests for now, not sure how to do it bests. @PVince81 @Xenopathic feel like reviewing 😄 |
Currently migration leaves the original Once everything is properly tested I would like to rename them to something like I don't want to remove them since that would remove any chance of (manual) recovery in case of migration errors |
5fe7207
to
f302c79
Compare
@icewind1991 Update also the title and the labels if this is ready for review. Thanks :) |
@icewind1991 sounds good |
f302c79
to
47ad430
Compare
I think this is ready for review, there's still work to be done but it shouldn't cause any regressions and it's easier to work on the remaining stuff in smaller bits |
$applicableGroups[] = $applicable; | ||
$storageConfig->setApplicableGroups($applicableGroups); | ||
} | ||
return $storageConfig; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this was simply moved from the existing code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
The code looks good, missing a few PHPDocs for clarification. 👍 |
@icewind1991 please set to "To review" after you updated according to the comments. |
Added phpdocs |
Needs a second reviewer @Xenopathic @DeepDiver1975 |
Files external test plan by @davitol owncloud/QA#39 Please note that for this PR only the storage configuration needs to be tested (with updates), no need to test all operations of the storages themselves. As long as the storage is mounted properly and upload works, then it's fine. |
👍 |
Oh, there's an Oracle issue... |
88852bb
to
a1898dc
Compare
Oracle fail:
|
Doesn't look directly related, but also doesn't seem to fail on master. |
https://ci.owncloud.org/job/core-ci-linux/database=oci,label=SLAVE/13045/ says oci passed... |
Okay, maybe it was the old one... I clicked on Details while it was still running. |
Move files_external mount config to the database
This probably broke ownCloud mounts: #22605 |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
TODO:
cc @PVince81 @Xenopathic